@Lightheartdevs Thanks for the thorough review! I adjusted the PR.

1. jq auto-install - Good catch. I've added auto-install logic for jq.
2. CI - Pushed an adjustments commit, this should trigger the CI pipeline
3. Docker Compose GPU reservation - In fact this is not a conflict, the current setup is intentional and correct, as device_ids in the deploy.resources.reservations.devices block can't be set dynamically, because Docker Compose variable interpolation only produces scalar strings, and since device_ids expects a YAML sequence, there's no way to inject a list like ['0', '2'] from an environment variable.
The two mechanisms are layering, ie. the deploy reservation makes all GPUs available at the Docker level, for the NVIDIA container runtime to use NVIDIA_VISIBLE_DEVICES to scope which GPUs are actually visible inside the container at the runtime level.
This is a common approach when you need dynamic per-container GPU assignment in Compose.

4. INSTALL_START_EPOCH duplication - Fixed!

I appreciate the detailed feedback!

Review Update — Rebase Required Before Merge

Hey @y-coffee-dev, great work addressing the previous review items. The code itself is solid and we want to get this merged. However, we found a critical issue that needs attention first.

🚨 Silent merge bug: LLM_MODEL_SIZE_MB will be dropped

Since you branched, we merged #572/#573/#574 which rewrote the model names and URLs in tier-map.sh (Qwen 3 → Qwen 3.5). Your branch adds LLM_MODEL_SIZE_MB to each tier in that same file.

Git reports a clean merge — no conflicts — but the result silently drops all 11 of your LLM_MODEL_SIZE_MB additions. This happens because git sees main's rewrites and your additions as non-overlapping changes within each tier block, and resolves by taking main's version (which has no LLM_MODEL_SIZE_MB).

What breaks: assign_gpus.py gets called with --model-size "" → float("") → ValueError → multi-GPU assignment fails on every install. Single-GPU installs are fine (early return guard), but the entire multi-GPU feature would be DOA.

What's needed

1. Rebase onto current main (commit 5a932e9)
2. Re-add LLM_MODEL_SIZE_MB to each tier. The new Qwen 3.5 model sizes (update as needed):

CLOUD:      LLM_MODEL_SIZE_MB=0
ARC:        LLM_MODEL_SIZE_MB=5760    # Qwen3.5-9B-Q4_K_M
ARC_LITE:   LLM_MODEL_SIZE_MB=2870    # Qwen3.5-4B-Q4_K_M
NV_ULTRA:   LLM_MODEL_SIZE_MB=48500   # Qwen3-Coder-Next-Q4_K_M (unchanged)
SH_LARGE:   LLM_MODEL_SIZE_MB=48500   # Qwen3-Coder-Next-Q4_K_M (unchanged)
SH_COMPACT: LLM_MODEL_SIZE_MB=18600   # Qwen3-30B-A3B-Q4_K_M (unchanged)
Tier 0:     LLM_MODEL_SIZE_MB=1500    # Qwen3.5-2B-Q4_K_M
Tier 1:     LLM_MODEL_SIZE_MB=5760    # Qwen3.5-9B-Q4_K_M
Tier 2:     LLM_MODEL_SIZE_MB=5760    # Qwen3.5-9B-Q4_K_M
Tier 3:     LLM_MODEL_SIZE_MB=16400   # Qwen3.5-27B-Q4_K_M
Tier 4:     LLM_MODEL_SIZE_MB=18600   # Qwen3-30B-A3B-Q4_K_M (unchanged)

⚠️ Double-check these against the actual GGUF file sizes on HuggingFace — the Qwen 3.5 models are new and some sizes may differ from the Qwen 3 equivalents you had before.

3. Push — this should also trigger CI, which hasn't run yet on this branch.

Everything else looks good

We did a full merge simulation and traced every touched installer file. The single-GPU path is completely safe — your guards in 02-detection.sh (GPU_COUNT -gt 1) and 03-features.sh (GPU_COUNT -le 1 → return) are clean. The compose layering, hardware detection additions, and .env generation all use safe defaults. No behavioral changes for existing single-GPU installs on any backend.

Two minor suggestions for a follow-up (non-blocking):

• Add trap "rm -f $TOPOLOGY_FILE" EXIT after the mktemp in 03-features.sh to clean up on early exit
• Add a # NOTE: keep in sync with assign_gpus.py comment in the custom TUI parallelism logic in 03-features.sh, since it duplicates the threshold logic from the Python script

Looking forward to the rebase — this is a great feature and we want to ship it. 🚀

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

Code Review — 2026-03-24

Overall: Well-architected PR. The topology detection, strategy selection, and compose overlay pattern are all clean and consistent with existing conventions. Good test coverage (7 fixtures). Not merging yet due to conflicts and testing requirements, but this is on track.

Merge Conflicts (5 files)

This PR was branched before the Lemonade integration (19 PRs merged 2026-03-24). The following files will conflict:

• 02-detection.sh — we added cpu/apple case handlers (fix(installer): CPU backend wrongly overridden to nvidia when capability profile loaded #596) and the tier assignment block was modified
• 06-directories.sh — heavily modified for Lemonade .env generation, LiteLLM config, DREAM_MODE
• constants.sh — VERSION bumped to 2.4.0
• .env.schema.json — several new keys added (DREAM_MODE, LLM_BACKEND, LLM_API_BASE_PATH, TARGET_API_KEY, OPENAI_API_KEY)
• resolve-compose-stack.sh — lemonade mode added to compose stack resolution

Action needed: Rebase onto current main and resolve conflicts. We can help with this if needed.

Revision Requests

1. Verify INTERACTIVE guard on manual GPU assignment. Phase 03 adds interactive prompts for custom GPU assignment. Confirm these are gated by [[ "$INTERACTIVE" == "true" ]] so non-interactive installs (CI, scripted, --yes) don't hang waiting for input.

2. jq is now a hard dependency. Phase 01 auto-installs jq if missing — this changes jq from optional to required. Acceptable given multi-GPU needs it, but:

   • Add jq to the README prerequisites list
   • Consider gating the auto-install behind GPU_COUNT > 1 so single-GPU users aren't surprised

3. LLM_MODEL_SIZE_MB — missing newline at EOF in .env.schema.json. The diff shows the trailing } lost its newline. Minor but fails some linters.

4. compose.multigpu.yml — NVIDIA_VISIBLE_DEVICES default. Currently defaults to all when LLAMA_SERVER_GPU_UUIDS is empty. On a multi-GPU system where assignment runs, this should always be set. But if the assignment fails silently, all is a safe fallback. Consider logging a warning when falling back.

5. Temp file cleanup. TOPOLOGY_FILE is created via mktemp in Phase 03 but never cleaned up. Add a trap or explicit rm at the end of the phase.

Testing Requirements

• Needs testing on actual multi-GPU hardware (we don't have any in the current test matrix)
• The 7 topology fixtures cover detection well, but end-to-end install → compose up → services running on assigned GPUs hasn't been validated
• Specifically: verify NVIDIA_VISIBLE_DEVICES with UUID list actually constrains the right containers

What's Good

• nvidia-topo.sh is a clean pure-function library — follows the lib/ conventions perfectly
• assign_gpus.py strategy engine is well-separated from bash
• Compose overlay pattern (compose.multigpu.yaml per service) is consistent with existing GPU overlays
• Test fixtures are thorough (1-GPU PCIe through 8-GPU NVLink full mesh with NUMA)
• LLM_MODEL_SIZE_MB in tier-map is a useful addition beyond multi-GPU
• NVLink vs PCIe strategy selection (tensor split vs pipeline) is the right approach

Status: Defer merge until conflicts resolved and multi-GPU hardware testing available. Happy to help with conflict resolution when ready.

Hey @Lightheartdevs! Thanks for the thorough review across installer files, really appreciate the care that went into this.

Good catch on the potential silent drop, I rebased and made sure all LLM_MODEL_SIZE_MB values survived.

Revision requests

INTERACTIVE guard: confirmed and added an explicit early-return guard at the top of run_custom() as an extra safety net, on top of the existing call-site gating.

jq: After I checked deeper, it turns out jq was already a hard dependency before this PR. scripts/validate-env.sh has been using it to parse .env.schema.json on every install regardless of GPU count. So single-GPU users were already getting it. Added it to the README prerequisites.

.env.schema.json newline + LLM_MODEL_SIZE_MB: fixed the trailing newline and added LLM_MODEL_SIZE_MB as a proper schema entry

NVIDIA_VISIBLE_DEVICES fallback warning: added a warn in phase 03 right after GPU assignment is extracted.

Temp file cleanup: added trap "rm -f $TOPOLOGY_FILE" EXIT and added an explicit rm -f "$TOPOLOGY_FILE" at the end of the phase so it cleans up promptly.

Hardware testing

I tested this end-to-end on several multi-GPU machines before, install through compose up, services running on assigned GPUs, and confirmed NVIDIA_VISIBLE_DEVICES with UUID lists actually constrains the right containers. That said, totally understand if you'd rather wait until the CI matrix has multi-GPU coverage before merging.

Thanks again for the detailed feedback!

Code Review — 2026-03-25

Solid multi-GPU feature. Architecture is clean, test coverage is thorough, single-GPU and AMD paths are unaffected. Ready to merge with minor notes:

Non-blocking observations

1. jq dependency — Phase 02 now requires jq for topology JSON parsing. README mentions it but Phase 01 preflight doesn't check for it. The installer already auto-installs jq per the README note, so this should be fine in practice.

2. base64 -w0 is GNU-specific (line 293, Phase 06). macOS base64 doesn't support -w0. Not a practical issue since multi-GPU is gated by GPU_BACKEND == nvidia and GPU_COUNT > 1 — no Mac will hit this path. But if anyone adds AMD multi-GPU later, this needs a portable wrapper.

3. Custom mode duplicates assign_gpus.py parallelism logic in bash (Phase 03, ~line 240). Comment says "keep in sync." Consider calling assign_gpus.py with a custom topology JSON from the custom assignments instead, to avoid divergence.

4. LLM_MODEL_SIZE_MB — verify this is always set before Phase 03 runs. If the tier map doesn't populate it, the assignment algorithm will error.

What's good

• Topology fixture coverage is excellent (8 configs from 1-GPU to 8-GPU NVLink mesh)
• Graceful degradation on detection failure (warn, skip, continue)
• Compose overlay pattern is consistent with existing AMD/CPU/Apple overlays
• Interactive custom mode gives power users full control
• Single-GPU installs see zero change

Approved. ✅